- 
                Notifications
    
You must be signed in to change notification settings  - Fork 292
 
archiver: fix block public access setting to true (aka 'set to private') #2409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the S3 bucket creation script to properly make buckets private by enabling all public access blocking settings. Previously, the script was incorrectly creating publicly accessible buckets.
- Changes all public access block configuration values from 
falsetotrue 
Comments suppressed due to low confidence (1)
monad-archive/scripts/create-bucket.sh:26
- The comment on line 28 is now misleading. It says 'Disabling block public access settings...' but the code is actually enabling block public access settings to make the bucket private. This should be updated to 'Enabling block public access settings...' or 'Making bucket private...'.
 
echo "Disabling block public access settings..."
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # Disable block public access | ||
| aws s3api put-public-access-block \ | ||
| --bucket "$S3_BUCKET_NAME" \ | ||
| --public-access-block-configuration "BlockPublicAcls=false,IgnorePublicAcls=false,BlockPublicPolicy=false,RestrictPublicBuckets=false" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script is primarily for creating archive replica buckets, which generally should be public by default. Consider either add a cli flag for non-public
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related question:
if we are making a public bucket, do we want to ensure that it's requestor pays?
I would assume "yes" which means:
- public -> requestor pays
 - private (doesn't matter about requestor)
 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want requester pays on all buckets actually, since if they're internal we pay pay either way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update the script to have the switch and set the buckets to requestor pays
Backstory: